Skip to content

Refactor: 회원가입 페이지 컴포넌트 리팩토링 : RSC, RCC 분리, async-await문으로 리팩토링 #77

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Aug 12, 2024

Conversation

guesung
Copy link
Collaborator

@guesung guesung commented Aug 8, 2024

What is this PR? 🔍

  • 회원가입 페이지 컴포넌트 리팩토링

Changes 📝

  1. RSC와 RCC의 분리를 위해 각 컴포넌트를 분리하였습니다. 2b4d1c5 0f01e6e 2e72d3d

여기서는, 모두 클라이언트 컴포넌트라 유의미한 변화가 보이지 않아보이지만, 다른 페이지 컴포넌트에서는 더 유의미한 변화를 확인할 수 있을 것입니다.

image

예시로, pick페이지에서 빨간색으로 칠한 부분만 RCC(React Client Component)로, 그 외에는 RSC(React Server Component)로 살릴 수 있을 것입니다.

앞으로 이 방향으로 다른 페이지도 마이그레이션을 하고자합니다. 선영님 의견이 궁금해요 !

  1. .then문을 async-await문으로 수정했습니다.

.then문보다 async-await문이 더 선언적이라 가독성이 좋다고 생각하여 이 방향으로 수정을 하였는데, 이러한 컨벤션이 정해진 게 아직 없는 거 같아요. 선영님 의견이 궁금합니다.

가독성 뿐 아니라, async-await는 비동기 처리에 대한 결과값을 변수에 담을 수 있기에 비동기를 '일급'으로 다룰 수 있다는 점에서 더 유연한 방식이라고 생각합니다.

getData().then(response => {})
const response = await getData();

@guesung guesung self-assigned this Aug 8, 2024
@guesung guesung changed the title Refactor: 회원가입 로직 리팩토링 Refactor: 회원가입 페이지 컴포넌트 리팩토링 : RSC, RCC 분리, async-await문으로 리팩토링 Aug 8, 2024
Copy link
Member

@seondal seondal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

친절하게 PR에 설명 남겨주셔서 감사합니다. 많이 배워갑니다..!
폴더구조 관련해서 코멘트 남긴 부분 확인 부탁드려요 :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

한가지 컴포넌트로 이루어진 페이지를 만드는 이유를 잘 모르겠습니다..!
가능하면 파일 갯수를 줄이고 싶은데, 본 페이지에 LogoutComponent 코드를 넣는건 어려울까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

컨벤션을 맞추기 위해서 따로 뺐습니다.

모든 페이지는 RSC(React Server Component)로 두고, 그 하위에 RSC와 RCC(React Client Component)로 구분해야 추후 확장에 있어서 RSC와 RCC를 구분하여 컴포넌트를 추가할 수 있기 때문입니다.

만약 이 page.tsx 컴포넌트가 RCC이면, 추후 RSC로 구현이 가능한 컴포넌트가 추가되었을 때 구조를 다시 바꿔야 한다는 불편함이 예상됩니다. 그래서 모든 page.tsx컴포넌트는 RSC로 두고, 하위 컴포넌트에서 RSC와 RCC를 구분하고자 하는 컨벤션을 사용하는 건 어떨까요?

의견 자유롭게 제시해주세요 !!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

모든 페이지를 RSC로 두는 것에 대해서는 찬성합니다..!

폴더구조에 대해서, page.tsx도 하나의 컴포넌트인 만큼 해당 페이지에 종속되는 컴포넌트들은 같은 폴더 내에 (components 폴더를 매번 따로 만들지 않고) 정의하는 방식으로 폴더의 depth를 줄여나가면 더 좋을 것 같습니다!

Copy link
Collaborator Author

@guesung guesung Aug 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

링크

좋습니다 :) 구두로 논의한 방식으로 컨벤션에 추가했습니다 !

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7d2acb3 수정한 커밋입니다 !

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

557e50a 추가된 컨벤션에 맞춰서 리팩토링 진행했습니다!

@guesung guesung changed the base branch from main to develop August 12, 2024 05:22
@dnd-side-project dnd-side-project deleted a comment from github-actions bot Aug 12, 2024
@dnd-side-project dnd-side-project deleted a comment from github-actions bot Aug 12, 2024
@dnd-side-project dnd-side-project deleted a comment from github-actions bot Aug 12, 2024
@dnd-side-project dnd-side-project deleted a comment from github-actions bot Aug 12, 2024
@dnd-side-project dnd-side-project deleted a comment from github-actions bot Aug 12, 2024
@guesung
Copy link
Collaborator Author

guesung commented Aug 12, 2024

@seondal 머지할까요 ??

* ♻️ refactor: `isServer` 분리

* ♻️ refactor: client 접근 cookie와 server 접근 cookie 분리

* ♻️ refactor: 토큰 관련 로직 privateApi에서 일괄 처리

* ♻️ refactor: 토큰값을 쿠키에서 가져오기

* ♻️ refactor: ACCESS_TOKEN으로 accesstoken 상수화

* ♻️ refactor: 불필요한 logout 페이지 제거

* ♻️ refactor: privateApi에서 로그아웃 처리

* ✨ feat: error status 모듈화

* 🐛 fix: cookie가 없을 경우, throw하지 않고 빈 값을 리턴한다.

* 🐛 fix: 로그인 페이지로 다시 돌아가는 이슈 해결

* ♻️ refactor: 토큰관리를 기존 localStorage & recoil에서 cookie로 마이그레이션

* ♻️ refactor: 추상화 수준 통일

* ♻️ refactor: 로그아웃 로직 수정

* refactor: 사용자의 정보를 쿠키로 마이그레이션

* ♻️ refactor: email과 nickname 상수화

* feat: 415에러도 추가
@guesung guesung merged commit 470c6e8 into develop Aug 12, 2024
1 check passed
@guesung guesung deleted the refactor/refactor-the-auth branch August 12, 2024 07:03
accesstoken: string,
options?: UseInfiniteQueryOptions<PoseFeedContents>
) =>
export const useBookmarkFeedQuery = (options?: UseInfiniteQueryOptions<PoseFeedContents>) =>
useSuspenseInfiniteQuery<PoseFeedContents>(
['bookmarkFeed'],
({ pageParam = 0 }) => getBookmarkFeed(pageParam),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치를 간단히 코드 리뷰해드리겠습니다.

고쳐야 할 부분:

  1. useBookmarkFeedQuery 함수의 인자로 accessToken을 요청하는 것이 사라졌는데, 이는 기존 코드와 호환성 문제를 일으킬 수 있습니다.
  2. options 매개변수가 선택적으로 변경되었는데, 해당 변경에 대한 적절한 설명 또는 주석이 필요합니다.

개선 제안:

  1. accesstoken 매개변수가 완전히 없어진 부분에 대한 변경사항을 충분히 문서화하거나 코멘트 달아주면 좋습니다.
  2. 코드 변경에 따른 테스트를 실행하여 기존 동작의 변화를 확인하는 것이 바람직합니다.
  3. 코드베이스 전반적으로 문자열 상수 및 직접 작성한 문자열을 상수로 추출하여 가독성을 향상시킬 수 있습니다.

@dnd-side-project dnd-side-project deleted a comment from github-actions bot Aug 12, 2024
export default function BookmarkSecion({ accesstoken }: BookmarkSecionI) {
const query = useBookmarkFeedQuery(accesstoken);
export default function BookmarkSecion() {
const query = useBookmarkFeedQuery();

return (
<FeedSection query={query}>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 코드 리뷰 요약:
  • 사용되지 않는 'use client'; 라인 제거 필요
  • BookmarkSecion 컴포넌트의 props 설정에서 accesstoken을 인자로 받도록 되어 있는데, 실제로 이 값을 사용하지 않음
  • BookmarkSecion 컴포넌트가 BookmarkEmpty, FeedSection 컴포넌트를 import하고 있는데, 실제로 사용되는 방식이 아님
  1. 버그 및 개선 제안:
  • 'use client'; 라인은 필요 없으므로 삭제하세요.
  • BookmarkSecion 컴포넌트의 BookmarkSecionI interface와 accesstoken prop은 현재 사용되지 않으므로 제거하는 것이 좋습니다.
  • BookmarkEmptyFeedSection 컴포넌트가 불필요하게 import된 것 같습니다. 사용되지 않는 코드는 정리해 주세요.

좀더 간결하고 효율적인 코드를 위한 변경 사항들을 제안 드렸습니다.

@dnd-side-project dnd-side-project deleted a comment from github-actions bot Aug 12, 2024
if (token) return <BookmarkSecion accesstoken={token?.accessToken} />;
else return <BookmarkEmpty />;
if (token) return <BookmarkSecion />;
return <BookmarkEmpty />;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 버그 리스크:

    • 코드 패치에서 import BookmarkSecion from './BookmarkSecion';가 아닌 import BookmarkSection from './BookmarkSection';으로 수정해야 합니다.
    • 반환되는 컴포넌트가 <BookmarkSecion />이지만, 실제로 accessToken prop을 제공하지 않으므로 해당 부분을 확인하고 조치해야 합니다.
  2. 개선 제안:

    • 에러 처리를 추가하여 사용자 토큰 확인 중에 발생하는 예외 상황을 처리할 수 있도록 하십시오.
    • 비동기 함수 BookmarkPage의 오류 처리를 개선하고, 적절한 오류 메시지를 반환하도록 수정하세요.
    • 코드 읽기 쉽게 만들기 위해 변수나 함수명을 명확하게 지정하여 가독성을 높일 필요가 있습니다.

<span id="subtitle-1">로그아웃</span>
</div>
<Link href={'/menu/withdraw'} className="cursor-pointer py-12">
<Link href={'/menu/withdraw'} className="py-12 cursor-pointer">
<span id="subtitle-1" className="text-tertiary">
탈퇴하기
</span>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드의 리뷰를 짧게 작성해 드리겠습니다.

  1. 개선 사항:
  • 클라이언트 측에서 민감한 데이터(예: 액세스 토큰)를 사용하는 것은 보안 위험을 초래할 수 있습니다.
  • handleLogout 함수에 대한 확실한 에러 핸들링이 필요합니다.
  • menuList를 사용하지 않는다면, 해당 import 문은 삭제되어야 합니다. 불필요한 코드로 남아있습니다.
  • 사용되지 않는 LogoutModal import도 삭제하는 것이 좋습니다.
  1. 버그 리스크:
  • 토큰이 sessionStorage나 localStorage에 저장되는 경우, 보안 문제가 발생할 수 있습니다.
  • Link 컴포넌트를 사용할 때 href 속성에 object를 제대로 전달하지 않고 있습니다. 이를 수정해야 합니다.
  1. 일반적인 권장:
  • React 컴포넌트에 대한 단위 테스트(Test Code) 작성이 부족합니다.
  • Consistency를 위해 CSS 클래스 순서를 통일하면 더 읽기 쉬울 수 있습니다.

개선을 위해:

  • 민감한 데이터에 대한 보안 강화
  • 안정적인 에러 핸들링 추가
  • 불필요한 import 및 코드 정리
  • 보안 상 스토리지에 민감한 데이터를 저장할 때 주의

등을 고려하면 좋을 것입니다.

return (
<div className="px-20">
<Header close={true} title="메뉴" />
<Header close title="메뉴" />
<LoginSection />
<MenuListSection />
</div>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치의 개선점과 잠재적 버그 위험에 대한 간단한 코드 리뷰입니다:

  • use client 라인이 삭제되었습니다. 이것이 의도된 것이라면 적절한 조치입니다.
  • export default async function MenuPage()로 변경되었습니다. 비동기 함수로 변경됨에 따라 추가된 비동기 작업에 대한 주의가 필요할 수 있습니다.
  • <Header close={true} title="메뉴" /> 부분을 <Header close title="메뉴" />로 변경하여 props를 명시적으로 전달하는 대신 생략 기능을 사용하였습니다. 해당 변경은 깔끔하게 읽힙니다.
  • 현재 코드에서 특별한 버그 위험이 보이지는 않습니다. 다만, 컴포넌트 간 데이터 흐름이나 비동기 작업 등에 대한 안정성을 고려해야 할 수 있습니다.
  • 코드 스타일 일관성을 위해 모든 파일에서 같은 따옴표 사용을 유지하는 것이 좋습니다 (예: ' 또는 " 중 하나만 사용).

코드는 보호해야할 특별한 문제점은 없어 보입니다.

});

return null;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. import 구문에서 'next/navigation'이 아니라 'next/router'를 사용해야 합니다.
  2. useDidMount 대신 useEffect 훅을 사용하여 비동기 작업을 처리할 수 있습니다.
  3. 예외 처리에서 router.push('/')를 계속 반복해서 사용하는 대신, 에러 발생 시 특정 메시지를 보여주는 방안을 고려할 수 있습니다.
  4. 클라이언트 쪽 쿠키 설정에 대한 검증 및 보안을 강화할 필요가 있습니다.
  5. 코드에 주석을 추가하여 기능 및 처리 방법을 명확히 설명하는 것이 좋습니다.

});

return null;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치의 간단한 코드 리뷰를 제공하겠습니다.

  1. import { useRouter, useSearchParams } from 'next/navigation'; 이 부분에서 'next/navigation' 모듈을 사용하는 것은 잘못된 경로입니다. 올바른 경로인 'next/router'를 사용해야 합니다.

  2. useDidMount 훅은 내장 라이브러리에 없는데, 커스텀 훅으로 추측됩니다. 해당 훅이 어떻게 구현되었는지 점검할 필요가 있습니다.

  3. 비동기 작업을 수행하는 useDidMount 훅 내에서 에러 처리를 하지 않고 있습니다. 작업이 실패하면 적절한 방법으로 처리하는 코드가 빠져있습니다.

  4. patchLogout 함수가 정의되어 있지 않아 별도의 로그아웃 처리가 이루어지지 않습니다. 이에 관련된 수정이 필요합니다.

  5. router.replace('/menu');는 회원 탈퇴 직후에 실행되므로 사용자 경험 면에서 즉각적인 리디렉션에 대한 고민이 필요할 수 있습니다.

  6. 보안 측면에서 클라이언트 쿠키 값을 단순히 삭제하기만 하는 것보다 안전한 방식으로 처리하는 것이 좋습니다.

  7. 코드 리팩터링 시, 주석에 오타가 있습니다. "때"의 한자 표기 오류입니다.

  8. WithdrawComponent 함수는 JSX 엘리먼트를 반환하지 않음에 유의해야 합니다. 기능적으로 return null;이 의도대로 동작하는지 확인할 필요가 있습니다.

개선 사항:

  • 모듈 경로 수정 (next/navigation -> next/router)
  • 비동기 작업의 에러 처리 추가
  • patchLogout 함수 관련 처리 추가
  • 클라이언트 쿠키의 안전한 삭제 처리 추가
  • 사용자 리디렉션 부분에 대한 더 나은 해결책 검토 및 적용
  • useDidMount 훅의 구체적인 구현 리뷰 및 개선

주의: 위의 제안은 코드 리뷰의 일부로, 실제 상황에 맞게 구체적으로 작업을 진행해야 합니다.


interface BookmarkButtonI {
poseId: number;
isMarked: boolean;
}
export default function BookmarkButton({ poseId, isMarked }: BookmarkButtonI) {
const { open } = useOverlay();
const { token } = useUserState();
const token = getClientCookie(ACCESS_TOKEN);
const [marked, setMarked] = useState(isMarked);

function onClick() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰:

  1. import 구문에서는 상대 경로(@/) 대신 모듈의 절대 경로를 사용하는 것이 좋습니다.
  2. BookmarkButtonI 인터페이스의 이름은 구체적이라서 이해하기 쉽습니다.
  3. BookmarkButton 함수 컴포넌트에서 token을 가져오는 방식은 개선이 필요합니다. useUserState를 사용하던 대신 직접적인 방법을 택함으로써 코드의 의존성과 가독성을 높일 수 있습니다.
  4. useState를 사용하여 marked, setMarked를 선언한 것이 좋습니다.
  5. onClick 함수가 선언되어 있지만, 해당 함수가 실제로 어떻게 구현되는지는 코드 스니펫에 없습니다. 이 부분을 확인해 주세요.
  6. 쿠키 관련 작업을 수행하는 함수인 getClientCookie와 토큰 관련 상수인 ACCESS_TOKEN이 모듈 외부에서 가져올 때 어떻게 동작하는지에 따라 보안 및 에러 처리를 고민하셔야 합니다.

{isLogin
? `${userData?.nickname}님 환영합니다! 새 포즈를 등록해 보세요 :)`
{token
? `${nickname}님 환영합니다! 새 포즈를 등록해 보세요 :)`
: '간편 로그인으로 3초만에 가입할 수 있어요.'}
</div>
</div>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드는 개선할 부분이 있습니다.

  1. DefaultProfile 컴포넌트를 삭제하고 해당 JSX를 직접 LoginSection 내부에 추가합니다.
  2. ICON.profile에 대한 아이콘 크기를 하드 코딩하지 말고 변수로 관리합니다.
  3. email, nickname, token 변수들을 세팅하는 방식을 리팩토링하여 중복 코드를 줄입니다.
  4. 조건부 렌더링 시, 삼항 연산자 대신 조건문을 사용하여 가독성을 향상시킵니다.
  5. CSS 클래스명을 보다 의미있게 작성하고 일관된 방식으로 정렬합니다.

개선을 위해 위의 지적들을 고려하여 코드를 업데이트하세요.

<PrimaryButton text="로그인 유지" onClick={exit} />
</>
<PrimaryButton text="로그아웃" type="secondary" onClick={handleLogout} />
<PrimaryButton text="로그인 유지" onClick={exit} />
</Popup>
);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 리뷰의 결과는 다음과 같습니다:

  1. import { ACCESS_TOKEN } from '@/constants';: 액세스 토큰 상수를 가져오는 것은 좋은 접근 방식입니다.

  2. import { removeClientCookie } from '@/utils';: 클라이언트 쿠키를 제거하는 함수를 가져오는 것은 올바른 접근입니다.

  3. removeClientCookie(ACCESS_TOKEN);: 이 코드에서, 액세스 토큰을 이용하여 클라이언트 쿠키를 삭제하는 것이 안전한지 확인해야 합니다. XSS(Cross-Site Scripting) 공격에 취약할 수 있습니다. 쿠키를 안전하게 처리하기 위한 추가적인 보안 검토가 필요합니다.

  4. <PrimaryButton text="로그아웃" type="secondary" onClick={handleLogout} />: 로그아웃 버튼에 대한 클릭 핸들러를 개선하여 일관성 있는 로그아웃 동작을 보장할 수 있습니다.

  5. router.push('/');: 로그아웃 시 홈페이지로 이동하는 것은 일반적으로 권장되는 방법입니다.

  6. 잠재적인 개선 사항: 사용자가 실수로 로그아웃할 때 데이터 손실을 방지하기 위해 경고 메시지나 확인 대화 상자를 통해 사용자의 의도를 더 명확히 할 수 있음.

코드의 보안 측면을 강조하여 점검하는 것이 중요하며, 사용자 경험 측면에 대해서도 고려하는 것이 좋습니다.

@@ -0,0 +1,3 @@
export const ACCESS_TOKEN = 'accesstoken';
export const EMAIL = 'email';
export const NICKNAME = 'nickname';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 코드는 비교적 간단한 상수를 정의하고 있는 것으로 보입니다. 코드 리뷰 내용은 다음과 같습니다:

  1. 주석: 코드 상단에 상수들에 대한 간단한 설명이 추가되면 이해를 돕을 수 있습니다.
  2. 상수 이름: 전역 상수를 사용할 경우 네임스페이싱에 유의하여 특정 파일이나 모듈과 겹치지 않도록 신중히 선택하는 것이 좋습니다.
  3. 보안: 민감한 정보인 경우, 접근 권한 관리나 저장 방법에 대한 고려가 필요합니다.
  4. 타입 지정: TypeScript를 쓰는 경우 변수 형(type)을 명시하는 것이 유용할 수 있습니다.

개선 제안:

  1. 주석 추가: 각 상수에 대한 역할이나 용도에 대한 주석을 달아보세요.
  2. 네임스페이스: 각 상수가 어떤 모듈이나 파일에서 사용되는지 고려하여 변수명을 변경해보세요.
  3. 타입스크립트: TypeScript를 사용하면 코드의 안정성을 높일 수 있습니다. 타입 지정을 고려해보세요.

이 코드 자체적으로 버그 위험이 크게 없는 것으로 보입니다.

@@ -0,0 +1,2 @@
export const ERROR_UNAUTHORIZED = 401; // https://developer.mozilla.org/ko/docs/Web/HTTP/Status/
export const ERROR_UNSUPPORTED_MEDIA_TYPE = 415; // https://developer.mozilla.org/ko/docs/Web/HTTP/Status/415

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치는 간단한 상수를 내보내는 것으로 보이며, HTTP 상태 코드에 대한 설명 링크가 제공되고 있습니다. 코드 자체에는 큰 문제가 없어 보이지만 몇 가지 개선 제안이 있습니다:

  1. 주석이 상수에 대한 설명을 제공하기는 하지만, 상수 이름 자체가 설명적이지 않으므로 더 명확한 이름을 선택하는 것이 좋습니다.
  2. 상수를 사용하는 곳에서 상수를 활용하는 함수나 클래스와 같은 구조체를 함께 제공하여 코드의 가독성을 향상시킬 수 있습니다.

이러한 개선 사항들을 고려하여 코드를 더욱 유지보수하기 쉽고 확장 가능하도록 개선할 수 있습니다.

@@ -0,0 +1,2 @@
export * from './auth';
export * from './error';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치에서는 두 개의 export 문을 사용하여 'auth''error' 파일에서 내보낸 것을 외부로 내보내고 있습니다. 이러한 방식은 모듈에서 함수, 변수 또는 클래스 등을 내보내기 위해 사용됩니다.

장단점:

  1. 장점: 코드가 간결하며 편리하게 여러 모듈에서 같은 이름으로 내보낼 수 있습니다.
  2. 단점: 같은 이름으로 여러 모듈에서 내보내는 경우 충돌이 발생할 수 있습니다.

개선 제안:

  1. 와일드카드(*)를 사용하는 대신에 명시적인 이름을 사용하여 개별 요소를 내보내는 것이 좋습니다.
  2. 와일드카드(*)를 사용할 때는 기능 및 객체 간 충돌을 방지하기 위해 필요한 처리를 추가해야 합니다.

위의 사항들을 고려하여 코드를 검토해보세요.


export const removeClientCookie = (key: string) => {
document.cookie = `${key}=; expires=Thu, 01 Jan 1999 00:00:10 GMT;`;
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰:

  1. setClientCookie 함수에서의 options.expires는 Date 객체를 사용하도록 설정되어 있지만, 이 값이 전달되지 않을 수 있습니다. 이 경우에 대한 예외 처리가 필요할 수 있습니다.
  2. getClientCookie 함수에서는 쿠키 값을 가져오기 위해 직접적으로 정규식이나 안전한 방법을 사용하는 것이 더 바람직할 수 있습니다.
  3. removeClientCookie 함수에서 쿠키 삭제 시간을 "Thu, 01 Jan 1999 00:00:10 GMT"로 하고 있습니다. 실제로 쿠키를 삭제하기보다는 만료일을 현재 시간 이전으로 설정하는 것이 더 안전합니다.

위의 점을 고려하여 코드를 보완하시기를 권장합니다.

export const removeServerCookie = async (key: string) => {
const { cookies } = await import('next/headers');
cookies().set(key, '');
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 개선 사항:

    • setServerCookie 함수에서 expires 값이 옵셔널한데, 만약 이 값을 따로 설정하지 않으면 어떻게 동작할지에 대한 설명이 필요합니다.
    • 코드에서 외부 모듈을 가져오는 부분이 async 함수 안에서 처리되고 있지만, 재사용성 측면에서 해당 부분을 밖으로 빼내는 것이 더 좋을 수 있습니다.
  2. 버그 리스크:

    • removeServerCookie 함수에서 cookies().set(key, '');를 사용했는데, 이것은 쿠키를 제거하지 않고 빈 문자열로 설정하는 것일 수 있으니 쿠키 삭제 동작을 정확히 하는지 확인이 필요합니다.
  3. 변경점:

    • 모든 함수들이 비동기 함수로 작성되어 있는데, 해당 함수가 필요로 하는 경우에만 비동기 처리가 필요한지 확인하고, 필요 없다면 동기 함수로 변경하는 것도 고려해 볼 가치가 있습니다.
  4. 추가 제안:

    • 각 함수에 대한 에러 핸들링을 추가하여 예외 상황에 대한 처리를 보강하는 것이 좋습니다.
    • 설명이나 주석을 추가하여 각 함수의 목적과 동작에 대한 이해를 돕는 것도 유익할 수 있습니다.
    • 쿠키 관련 기능이므로 관련된 단위 테스트를 작성하여 각 함수의 동작이 예상대로 되고 있는지 확인하는 것이 중요합니다.

@@ -0,0 +1,3 @@
export * from './cookie.server';
export * from './cookie.client';
export * from './isServer';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 코드 패치는 서버 및 클라이언트 환경에서 쿠키 처리 및 서버 여부 확인 관련 파일을 내보내기하는 것으로 보입니다. 코드 리뷰 시에 주의할 사항은 다음과 같습니다:

  1. 동일한 이름 충돌: 모듈들이 같은 이름을 갖고 있는지 확인하십시오.
  2. 순환 종속성: 모듈 간의 순환 종속성이 있는지 확인하여 무한 루프를 방지합니다.
  3. 보안 취약점: 쿠키 처리와 관련된 보안 취약점이 있는지 검토하세요.

개선 제안:

  • 명확한 기능 설명: 각 파일(예: cookie.server, cookie.client, isServer)의 역할과 책임을 명확히 문서화하십시오.
  • 테스트 추가: 특히 쿠키 처리와 서버 여부 확인과 관련된 기능에 대한 테스트를 추가하여 안정성을 확보하세요.
  • 모듈 크기 최적화: 필요한 경우 코드 분할을 통해 번들의 크기를 최적화할 수 있습니다.

왜 구현 중인가? 어떤 문제를 해결하고자 하는가? 이러한 질문을 고려하여 개발을 진행하면 좋습니다.

@@ -0,0 +1 @@
export const isServer = typeof window === 'undefined';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위 코드는 간단하고 명확합니다. 개선 제안 및 버그 위험 사항은 없습니다. 이 코드 조각은 클라이언트 측 또는 서버 측인지를 결정하기 위해 window 객체의 존재를 기준으로 하는 상태 변수 isServer를 내보냅니다. 이 코드는 일반적으로 안전하며, 해당 작업을 수행하기에 적합합니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants